-
Notifications
You must be signed in to change notification settings - Fork 92
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fixed string subID bug #1291
fixed string subID bug #1291
Conversation
Hello! 👋 Thanks for opening your first pull request here! |
will fix #1290 |
Thanks @Aaronearlerichardson! Could you please modify one of our tests to cover this case? Also, please feel free to add yourself to the list of authors, and add a changelog entry: https://github.com/mne-tools/mne-bids/blob/main/CONTRIBUTING.md#instructions-for-first-time-contributors |
@Aaronearlerichardson please let me know if you intend to work on the final points I mentioned in my comment above, and what your expected timeline would be. If not, somebody else could perhaps take over (as long as you supply a few of your details to list you in the authors lists) |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1291 +/- ##
==========================================
- Coverage 97.61% 97.46% -0.16%
==========================================
Files 40 40
Lines 8685 8780 +95
==========================================
+ Hits 8478 8557 +79
- Misses 207 223 +16 ☔ View full report in Codecov by Sentry. |
@sappelhoff hope this covers it |
🎉 Congrats on merging your first pull request! 🥳 Looking forward to seeing more from you in the future! 💪 |
Thanks a lot @Aaronearlerichardson :-)
I added a few more commits, but thanks for the start and the contribution in general |
PR Description
Merge checklist
Maintainer, please confirm the following before merging.
If applicable: